Tagging along as a reviewer. Linking to the wider Kibana issue discussing global keyboard shortcuts for visibility. I think we can use this as a starting point for consistent UX and to road test a11y.

Preview documentation changes for this PR: https://eui.elastic.co/pr_6036/

[CONSTANCE EDIT] Moving my original PR description to this comment to maintain conversation clarity

⚠️ WIP, opening for early staging QA/testing and feedback

@miukimiu Just opening this for a quick reference - I'd love to get your thoughts on the design, styling, etc:

1. Is this too large for a popover? Would a modal be better?
2. Does an EuiDescriptionList work best for this? Or would, e.g. a table or similar be better?
3. I don't love the "info" icon but I don't think the "keyboardShortcut" icon is much better, it looks a bit confusing. Is there any chance we could add a keyboard icon that looks maybe something like this?
   

Hi @constancecchen 👋🏽 ,

1. I think the popover works.
2. A basic table works well.
3. There was a discussion about the keyboardShortcut icon (Add keyboard shortcut icon #1573). And there were some attempts to add a keyboard instead of just keys and it didn't work out. So I think is better if we stick with the current keyboardShortcut.

For the keyboard shortcut what do you think of using the tag <kbd> instead of a <EuiBadge>? But I think we need better styles for the <kbd> tag which relates to this issue: #5016. I can prioritize this issue and add a new component.

I did some tests in a CodeSandbox and with what I suggested the popover would look like the following screenshot:

CodeSandbox design idea:
https://codesandbox.io/s/modest-jepsen-6xsj0v?file=/demo.js

Note
The kbd styles might change a little. But the styles were based on what we have in Figma: https://www.figma.com/file/RzfYLj2xmH9K7gQtbSKygn/Elastic-UI?node-id=13648%3A4134

Love it, thanks Elizabet! <kbd> definitely makes more sense. One quick note, I don't think the table needs to have a responsive mode as the popover is already very small. What do you think?

Re the keyboardShortcut icon, we chatted about this in sync today and it sounds like we may be revisiting it - I can use it for now and we can contemplate updating it in-place later.

One quick note, I don't think the table needs to have a responsive mode as the popover is already very small. What do you think?

@constancecchen I agree. The table doesn't need the responsive mode. I updated the CodeSandbox.

New icon

@constancecchen here are a few ideas for the new icon:

I'm not 100% convinced these icons work in 16px x 16px but let me hear your opinion.

The iInCircle icon

Another option would be to keep your initial idea of using the iInCircle icon. I think it works better than the old keyboardShortcut icon. Also, it aligns with other "product's help" across Kibana and Cloud as you can see in our In product help guidelines.

Just to show an example in Discover:

Prototype with the iInCircle and new icon

Here are a few mocks with both iInCircle and the new icon in the toolbar or footer: Figma prototype.

In the prototype, one of the ideas is to keep the icon in the toolbar.

Another idea would be to align with the markdown editor and keep the "help" in the footer:

Let me hear your thoughts.

I love it - I like the 6th keyboard icon the most personally! 🎉

I'd push against putting the keyboard shortcuts button on the bottom bar / next to the pagination because of how that affects screen reader users - they won't even get to that by the time they've finished interacting with the grid.

EDIT: Also worth mentioning, I'm not strongly against using the help icon, but I think I'd prefer a keyboard icon for EuiDataGrid specifically. This is for several reasons:

1. It really does only contain keyboard shortcuts, unlike the Kibana examples where the popover is explaining other things
2. EuiDataGrid has a specific need for defining keyboard shortcuts: normal screen reader and keyboard navigation (e.g. tab, ctrl+opt+arrow keys) does not work as normal / as expected. These keyboard shortcuts are not optional enhancements on top of existing behavior; it is necessary that we note that there is a specific way of interacting with EuiDataGrid, or we risk SR/keyboard users not knowing what to do.
3. We're trying to establish a pattern for Kibana to use for defining keyboard shortcuts in the future (Keyboard shortcut registry kibana#90515), and I think it makes sense that there will be more keyboard shortcut popovers in the future in Kibana

@1Copenut Hey Trevor! Before I switch this branch over to using a table layout format for the keyboard shortcuts, could I get your opinion on the screen reader experience of it?

<table>: https://codesandbox.io/s/modest-jepsen-6xsj0v?file=/demo.js
<dl>/<dt>/<dd>: https://eui.elastic.co/pr_6036/#/tabular-content/data-grid

I could definitely be wrong here, but I'm surprisingly not a super huge fan of the table experience. I think the screen reader ends up reading a lot of cruft like 'row X of Y' and mashes the column title + cell contents together and it leads to a more distracting experience with that information than without (the description list just reads out a '20 of 24' at the end of the item).

I could be totally wrong about that not being useful info though - WDYT?

Preview documentation changes for this PR: https://eui.elastic.co/pr_6036/

@constancecchen I'm leaning toward this as a description list too. It's subtle, but my thinking is that the description list is used to define key / value pairs. I feel our intended audience for the aural readback will be familiar with what each term means, so having table columns read each time could be extraneous.

The only thing I might suggest adding would be an aria-labelledby="HEADING_ID" to the DL tag that references a heading in the popover. That gives the list a unique name in SR Lists menus, and reinforces the relationship between the heading as proxy for table columns.

Awesome, thank you @1Copenut! 🙌

@miukimiu Are you OK with moving forward with using EuiDescriptionList instead of EuiBasicTable? It changes the visual appearance somewhat, but if you want I can try to get back to that visual appearance w/ CSS alone rather than a table structure. For what it's worth, I feel fairly strongly about this as the biggest beneficiaries of the keyboard shortcuts panel is keyboard/screen reader users 😅

EDIT: any custom CSS tweaking should perhaps wait until Bree is done converting EuiDescriptionList in #5971. This PR can definitely hang around until then, there's no rush!

Preview documentation changes for this PR: https://eui.elastic.co/pr_6036/

@miukimiu Are you OK with moving forward with using EuiDescriptionList instead of EuiBasicTable?

Yes, I'm ok with that. I'll update CodeSandbox to reflect that.

I'd like to make one quick suggestion on the formatting of the popover contents in an effort to make it more concise. Here's a screenshot:

1. Use the emoticon where possible to signify the correct key. It's much easier to scan for ↓ than arrow down.
2. Combine some of the simple & similar shortcuts in a comma separated list
3. Use bold format to highlight the key differences in the command
4. Try not to use terms like "popover" but instead "details"

Also, I'd label this "keyboard navigation" vs "shortcuts" because "shortcuts" assumes there's a menu item associated with the command. But really, this is just using the keyboard to traverse the elements.

If we want to think scalability, I'd probably opt for the iInCircle icon as the triggering button so that this popover could then contain more information in the future.

One item I'm still not clear on is the page up/down navigation. I can't tell if something is broken or why it's not another "focus" type of navigation.

Use the emoticon where possible to signify the correct key. It's much easier to scan for ↓ than arrow down.

I'm super digging this! Screen readers are reading the arrows out perfectly as well (although a quick note that we do need to use the actual unicode arrow characters vs emojis)

Combine some of the simple & similar shortcuts in a comma separated list

I'm not 100% confident about this but I strongly worry about the cognitive separation here for screen reader users, who will be navigating through 4 different arrow keys and then just "moves cell focus".

For what it's worth, when I separated each individual arrow key description, I was following W3's pattern of listing shortcuts: https://www.w3.org/WAI/ARIA/apg/patterns/grid/#keyboard-interaction-for-data-grids

Sadly W3 doesn't have a pattern/spec for listing keyboard shortcuts (https://www.w3.org/WAI/ARIA/apg/practices/keyboard-interface is fairly vague / doesn't mention how to tell consumers what your keyboard shortcuts are), so I don't have a lot of other examples/standards to reference, but as another example GitHub also separates out each arrow key: https://docs.github.com/en/get-started/using-github/keyboard-shortcuts#network-graph

@1Copenut, any thoughts?

Try not to use terms like "popover" but instead "details"

Sure, but for SR users we should somehow inform them that not all cells have details / are interactive (in case they press enter on a non-interactive cell and get nothing). It's probably also worth mentioning that cell actions are available in the details (because cell actions are not available or even mentioned to non-sighted keyboard users except through the interactive cell popover).

If you really want to be brief for sighted users, we could take advantage of SR-only text to do something like:

Opens cell details <EuiScreenReaderOnly>for interactive cells that contain cell actions or overflow data. These details open in a new dialog.</EuiScreenReaderOnly>.

Also, I'd label this "keyboard navigation" vs "shortcuts" because "shortcuts" assumes there's a menu item associated with the command. But really, this is just using the keyboard to traverse the elements.

Looks like https://www.w3.org/WAI/ARIA/apg/practices/keyboard-interface agrees with you - it primarily references keyboard shortcuts in terms of 'assigning and revealing' and otherwise uses 'keyboard navigation'.

Honestly though, I think that line is pretty blurry (e.g. looking at GitHub shortcuts again, many of these are navigation adjacent - although of course many aren't).

Re: page up/down navigation - it should be focusing the same cell column but in the last/next page's last/first row. Unfortunately W3 has no examples of data grids with pagination so we're deviating from their spec there / making up our own rules for page up/down.

If we want to think scalability, I'd probably opt for the iInCircle icon as the triggering button so that this popover could then contain more information in the future.

I would strongly rather keep the focus of this popover on keyboard accessibility. As I mentioned earlier, screen reader and keyboard users unfamiliar with the spec and standard keyboard interaction for data grids will potentially either not know or be unable to fully interact with the grid otherwise. The standard SR way of browsing through items will fail them for grids that scroll due to the virtualization library, as our grid only scrolls to cells out of view via arrow keys and not via SR arrow navigation.

It's also worth noting the custom logic I added to this button, while consumers can opt to 'hide' this button via toolbarVisibility.showKeyboardShortcuts: false, it only visually hides the button - keyboard users who tab in sequence through the data grid toolbar will always have these keyboard shortcuts available to them to view. I don't think this paradigm is necessary for other random information.

My goal with this PR was to establish a pattern that Kibana could reference or use in elastic/kibana#90515 and get ahead of a few issues (such as a few we've already solved, e.g. should we use a table or a description list? is the icon clearly readable as a keyboard?). While it's not 1:1 because EuiDataGrid doesn't exactly have 'shortcuts' or assigning shortcuts as you mentioned Caroline, nevertheless I still think it's worth keeping keyboard documentation as a first class citizen.

Combine some of the simple & similar shortcuts in a comma separated list

I'd opt to leave the list as it is now, with each navigation key / chord having its own definition. It's verbose but there's a clear relationship between key and value in each pair.

Also, I'd label this "keyboard navigation" vs "shortcuts" because "shortcuts" assumes there's a menu item associated with the command. But really, this is just using the keyboard to traverse the elements.

I concur this feels like navigation. If we use this nomenclature for data grids, we can extend and use "shortcuts" when we expand this UI into Kibana custom commands. At that point it really is a shortcut; we'd be removing steps between A and B.

If we want to think scalability, I'd probably opt for the iInCircle icon as the triggering button so that this popover could then contain more information in the future.

I second keeping the focus of this popover and its icon on keyboard navigation. The navigation pattern we're looking to establish is specifically for data grids and is wholly driven by key presses. Let's keep the ilnCircle for a Yes And when we start introducing keyboard shortcuts in Kibana.

It's a compromise between re-stating obvious/common navigational patterns and highlighting those that are unique. The cell expansion ones are the most unique but are at the bottom of the list most likely hidden by scroll if every item is listed by itself. We don't have to be quite as brief as Moves cell focus, but could be Moves cell focus up, down, left, right respectively.

I only mention the scalability because the toolbars are getting quite cramped and most of the time these keyboard commands charts are a one-time necessity, yet it'll be displayed on all (possibly multiple on one page) data grids all the time. I'm not saying they're not helpful but I'd be careful how close to and combined with actual data grid controls.

👋 Hey there. This PR hasn't had any activity for 90 days. We'll automatically close it if that trend continues for another week. If you feel this issue is still valid and needs attention please let us know with a comment.

Preview documentation changes for this PR: https://eui.elastic.co/pr_6036/

Preview documentation changes for this PR: https://eui.elastic.co/pr_6036/

@florent-leborgne can you help us with this PR?

This PR adds a keyboard shortcuts popover to the EuiDataGrid toolbar. The primary target audience of this functionality is low visual screen reader users who otherwise have no indication of how to interact with the data grid.

This is the current implementation that can be found here. To open the popover you just need to click the keyboard icon of the data grid:

Our questions are:

1. Should we be using the word "popover" or "details"? For example:

• "Opens cell expansion popover for interactive cells" or "Open cell details"
• "Closes any open popovers." or "Close cell details/Exits full screen"

2. Should the title of the popover be "Keyboard shortcuts" or "Keyboard navigation"?
3. Should we end each line with a dot?

Also, feel free to suggest other text changes.

Thanks @miukimiu for pinging me.

1. Should we be using the word "popover" or "details"? For example:

"Opens cell expansion popover for interactive cells" or "Open cell details"
"Closes any open popovers." or "Close cell details/Exits full screen"

I think details is much simpler. "Open/Close cell details" feels very simple in comparison to the full technical description of what really happens.

2. Should the title of the popover be "Keyboard shortcuts" or "Keyboard navigation"?

Keyboard shortcuts is fine. Small (maybe unrelated) question though: Those are for Windows (ctrl + home). Do you plan on displaying equivalent Mac shortcuts as well (usually fn+arrows for more advanced moves)?

3. Should we end each line with a dot?

I think no punctuation is fine. And no 3rd person needed as well. Here is what I came up with, with these changes and a few other minor tweaks:

Let me know what you think, I'm happy to iterate.

Thanks, @florent-leborgne for being so fast. All the changes look good to me. @constancecchen do you agree with the changes?

Small (maybe unrelated) question though: Those are for Windows (ctrl + home). Do you plan on displaying equivalent Mac shortcuts as well (usually fn+arrows for more advanced moves)?

I pass this question to @constancecchen. She's implementing the code.

Just for reference there are other shortcut sections in some places of the products (see screenshot as example).
Would this become the new standard for all shortcut references?

Just for reference there are other shortcut sections in some places of the products (see screenshot as example).
Would this become the new standard for all shortcut references?

@florent-leborgne we would like to enforce the usage of the EuiText <kbd> HTML element that was introduced in EUI v61.0.0. This element is used to indicate text that represents keyboard input from a user’s computer.

• This would ensure all shortcuts would visually look similar
• Also a better semantic usage

Would this information be a good candidate to be part of "in product assistance guidelines"?

Would this information be a good candidate to be part of "in product assistance guidelines"?

@miukimiu Yes totally!

One last suggestion on the text (thanks @gchaps); How about using bold format to highlight the key differences (like Caroline suggested earlier in this PR). I think it would help scanning the list a bit faster.

I apologize in advance if I appear argumentative, it's totally not my intention. But I want to stress a few things with this PR:

1. The goal of this PR is accessibility for non-sighted users. While visual/mouse users benefit from knowing keyboard shortcuts, they don't need this information in the same way that non-sighted users do. Screen reader users literally have no other guidance than this popover on how to correctly interact with the data grid, as the normal method of screen reader navigation in EuiDataGrid does not really work.
2. With that context in mind, while I appreciate the desire for brevity, I strongly think we need to err on the side of being explicit. Basically my litmus is, "if a blind user would not know otherwise know this information, do we need to spell out? Yes."
3. I want to mention that the majority of this copy I originally cribbed from W3 itself. W3C essentially sets the standard for accessibility and accessibility examples.

With that context in mind, I wanted to answer a few questions:

"Opens cell expansion popover for interactive cells" or "Open cell details" [...] I think details is much simpler. "Open/Close cell details" feels very simple in comparison to the full technical description of what really happens.

One minor objection - the cell expansion popover does not necessarily just contain more details. It also contains any truncated/overflowing cell actions that keyboard users literally cannot access outside of the expansion popover. The cell content itself is secondary to the fact that cell actions (i.e., interactive cells) must be accessed by keyboard user via popover, hence the stress on 'interactive cells'.

That being said, I get that it's wordy, and now that I write it out my main issue with the simplification, I think we can find a compromise. "Open cell details & actions" might sufficiently cover what I'm trying to convey 😅 WDYT? Any suggestions?

Those are for Windows (ctrl + home). Do you plan on displaying equivalent Mac shortcuts as well (usually fn+arrows for more advanced moves)?

For this I'm following W3's example which is only listing "Control". I think that might be because 90%~ of screen reader users are on Windows however (per WebAIM's 2021 survey). @1Copenut, any thoughts on this? Should we try to write a component that detects the user OS and displays Cmd instead?

(Ctrl) Home/End copy suggestions

"end/beginning of the row" is not enough information for a screen reader keyboard user. IMO, they need to know that it's the first cell/beginning of the row of the cell they are currently focused on (i.e., the current row). I don't think it's implied which row, otherwise. I also really think the ctrl descriptions are slightly misleading as it's not the first/last cell of the entire table (which may be paginated) - it's the first cell of the first row of the current page, and last cell of the last row of the last page.

I also personally prefer keeping the 'cell' copy between descriptions to make it clear that keyboard navigation centers around cells, but maybe that's just me.

How about using bold format to highlight the key differences (like Caroline suggested earlier in this PR). I think it would help scanning the list a bit faster.

Unfortunately this makes i18n translation a fairly annoying headache and I'm not confident it will translate well between languages. I would rather leave this alone (while screen readers do pick up emphasis, I'm not really sure that would provide all that much context to them). I don't feel very strongly about this however so am open to pushback if we really feel bolding is that useful.

Great discussion! I agree 100% with @constancecchen that explicit, clear explanations are our goal for this shortcut reference. This sets us up well for future shortcut menus when there's questions about clarity vs. brevity. I almost always come down on the side of verbosity and explicit instruction for assistive technology. Instructions that are present are easy to skip, but implied instructions are sometimes hard to deduce.

I'm quote answering the two items from comments above that I can add some insight.

That being said, I get that it's wordy, and now that I write it out my main issue with the simplification, I think we can find a compromise. "Open cell details & actions" might sufficiently cover what I'm trying to convey 😅 WDYT? Any suggestions?

This feels like a good compromise for clarity with brevity. If I was reading this with nothing else to go on, I could infer the cell had more information (details) and possibly interactions I could drive (actions).

Those are for Windows (ctrl + home). Do you plan on displaying equivalent Mac shortcuts as well (usually fn+arrows for more advanced moves)?

We won't need to add alternatives for the Ctrl + Home and Ctrl + End chords. Safari + VO on MacOS uses the Control key to activate these controls. Pressing Cmd + Home takes you to the Apple website by default in Safari.

Thank you so much @constancecchen and @1Copenut for the insights and background!

Open cell details & actions

This sounds good to me, maybe "Open cell details and actions", with plain text instead of &?

Safari + VO on MacOS uses the Control key to activate these controls.

Ok thank you! There are no obvious Home and End keys on my macbook so I had to Google for equivalents and I felt this could cause confusion.

the cell they are currently focused on (i.e., the current row). I don't think it's implied which row, otherwise.

All shortcuts are about moving from the current position so I really thought it would be OK to not repeat "current something" every time. I think we have 3 choices here: add it everywhere (I think it wouldn't help scanning), add it nowhere and consider it implied, or maybe add a small sentence at the top saying something like "You can navigate cells and change focus from its current position using the following shortcuts.".

I very much agree about your other points so I can make these new suggestions. Feel free to adapt them or use the ones from W3 as you ultimately have more background than me about best practices for accessibility:

• Move to the beginning of the row: Move to the first cell of the row
• Move to the end of the row: Move to the last cell of the row
• Move to the first cell of the table: Move to the first cell of the active page
• Move to the last cell of the table: Move to the last cell of the active page

Ultimately, if you feel like my suggestions strip out too much technical accuracy, we can go for the descriptions from W3, but I think that at least some parts are unnecessarily verbose compared to established grid/table navigation practices, and are even sometimes unclear if we go back to the point you made about "current focus" in your explanations. (for example, moves focus to the first cell in the first row. on W3 doesn't say if it's the first row from the current page either).

I would rather leave this alone (about bold)

This is a nice addition for many users even if they are not the primary target audience of this component. Nice doesn't mean critical as the list is still fairly short so I'll leave it up to you :)

Thanks again for all the great points!

@florent-leborgne does this current iteration of copy work for you? I personally prefer the specificity of including current as an adjective but would be open to changing it to something like active instead if you think it sounds better!

@constancecchen Your last suggestion looks good to me :) Thanks!

Thanks a million y'all!! I really appreciate all the discussion and guidance around this feature! @1Copenut and @miukimiu, do you mind doing a final approval pass?

Preview documentation changes for this PR: https://eui.elastic.co/pr_6036/

Thanks @miukimiu! Just to check, can I check "updated Figma library" off the PR checklist?

@constancecchen, yes! 👍🏽

@1Copenut Still waiting on your QA/review for this!

Woohoo, merging!! Thanks everyone!